Skip to content

Fix data migration#1661

Closed
nandi95 wants to merge 1 commit intoalexjustesen:mainfrom
nandi95:fix-data-migration
Closed

Fix data migration#1661
nandi95 wants to merge 1 commit intoalexjustesen:mainfrom
nandi95:fix-data-migration

Conversation

@nandi95
Copy link
Contributor

@nandi95 nandi95 commented Aug 11, 2024

📃 Description

There is an issue with the migrations when setting up the app for the first time. The server_id is missing.
#1564 (comment)

🔧 Fixed

  • database/migrations/2024_02_18_100000_results_bad_json_table.php

@nandi95 nandi95 requested a review from alexjustesen as a code owner August 11, 2024 15:19
@svenvg93
Copy link
Contributor

svenvg93 commented Aug 12, 2024

Hi @nandi95,
Just wondering is also server_location and isp not missing the migration?

  ->schema([
      Forms\Components\Placeholder::make('service')
          ->content(fn (Result $result): string => $result->service),
      Forms\Components\Placeholder::make('server_name')
          ->content(fn (Result $result): ?string => $result->server_name),
      Forms\Components\Placeholder::make('server_id')
          ->label('Server ID')
          ->content(fn (Result $result): ?string => $result->server_id),
      Forms\Components\Placeholder::make('isp')
          ->label('ISP')
          ->content(fn (Result $result): ?string => $result->isp),
      Forms\Components\Placeholder::make('server_location')
          ->label('Server Location')
          ->content(fn (Result $result): ?string => $result->server_location),
      Forms\Components\Placeholder::make('server_host')
          ->content(fn (Result $result): ?string => $result->server_host),
      Forms\Components\Checkbox::make('scheduled'),

@alexjustesen
Copy link
Owner

alexjustesen commented Aug 12, 2024

So the server_ fields were dropped from physical columns and instead reference the data in the data json column post "bad data" migration. This is why in places like the results table we reference the data column and then access attributes within the json structure:

https://github.com/alexjustesen/speedtest-tracker/blob/main/app/Filament/Resources/ResultResource.php#L174-L198

I don't think the cause of the issue is related to these fields, I'm going to close the PR as it looks like the form needs some updates instead.

@nandi95
Copy link
Contributor Author

nandi95 commented Aug 12, 2024

It is writing that property though

speedtest-tracker/app/Jobs/Speedtests/ExecuteOoklaSpeedtest.php

Lines 65 to 69 in 9a0d7da

$this->result->update([
'server_id' => $this->serverId,
'data' => json_decode($message, true),
'status' => ResultStatus::Failed,
]);

so maybe this needs to be updated? Either way currently this app doesn't function as a fresh install.

@alexjustesen
Copy link
Owner

alexjustesen commented Aug 12, 2024

Extracting to an issue so I can fix it tonight.

@alexjustesen alexjustesen mentioned this pull request Aug 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexjustesen alexjustesen Awaiting requested review from alexjustesen alexjustesen is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nandi95 @svenvg93 @alexjustesen